-
Notifications
You must be signed in to change notification settings - Fork 103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bug 1735728: Add description to operator user facing CRDs. #65
Conversation
@pliurh: This pull request references a valid Bugzilla bug. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@@ -53,7 +53,7 @@ type SriovNetworkNodeStateStatus struct { | |||
|
|||
// +genclient | |||
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object | |||
|
|||
// +kubebuilder:subresource:status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
curious, does updating nodestate work without this tag before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a bug in operator-sdk, which makes errors the generated CRD. Therefore, we have to do manual change to the generated CRD.
@@ -1,6 +1,6 @@ | |||
// +build !ignore_autogenerated | |||
|
|||
// Code generated by deepcopy-gen. DO NOT EDIT. | |||
// Code generated by operator-sdk. DO NOT EDIT. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did you use different tool this time to generate code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. It caused by the operator-sdk update.
@@ -10,8 +10,6 @@ spec: | |||
plural: sriovnetworknodepolicies | |||
singular: sriovnetworknodepolicy | |||
scope: Namespaced | |||
subresources: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so nodepolicy status is never used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct
maximum: 4096 | ||
minimum: 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I noticed the order of max/min vlan numbers have been changed many times, is there particular reason for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CRD file is auto generated by operator-sdk. I don't think the order matters.
@@ -26,22 +26,30 @@ spec: | |||
metadata: | |||
type: object | |||
spec: | |||
description: Specification describing how a NetworkAttachmentDefinition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we also change the file under manifests/4.2/manifests/4.2/sriov-network-operator-sriovnetwork.crd.yaml
?
before this PR, they have same content.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Will do.
/lgtm |
@pliurh: All pull requests linked via external trackers have merged. Bugzilla bug 1735728 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Allow modifying RESOURCE_PREFIX
No description provided.